@caseylocker This branch unconditionally calls fetchByItemRows(carried) every time the user switches into By Item, with no check for "already loaded, filters unchanged." Unlike the Orders/Lines branches above it (cheap single-page server fetches), this drives getPurchaseDetailsByItemRows, a whole-set bulk fetch (page 1, then the remaining pages via pLimit(10) at 100/page). Every Orders → By Item → Orders → By Item round-trip while exploring the report re-triggers that full multi-page burst against sponsor-reports-api, even when nothing changed.

Reproduced locally by mounting the page with byItemFilters: {} and byItemData already populated, then toggling orders → byitem → orders → byitem: getPurchaseDetailsByItemRows fires twice, even though the second entry has identical filters and already-loaded data.

// 1st toggle: orders -> byitem
fireEvent.click(screen.getByText("sponsor_reports_page.view_by_item"));
expect(getPurchaseDetailsByItemRows).toHaveBeenCalledTimes(1);

fireEvent.click(screen.getByText("sponsor_reports_page.view_orders"));
fireEvent.click(screen.getByText("sponsor_reports_page.view_by_item"));
// fetches again despite unchanged filters + already-loaded whole-set:
expect(getPurchaseDetailsByItemRows).toHaveBeenCalledTimes(2);

sameFilters(carried, byItemFilters) is already computed a few lines up to decide whether to reset paging — the same condition could gate the fetch itself: skip fetchByItemRows(carried) when byItemData.length > 0 && sameFilters(carried, byItemFilters).

Gated it. Re-entry now skips the whole-set fetch when the carried filters are unchanged and rows are already loaded, reusing the sameFilters(carried, byItemFilters) already computed for the paging reset. I also added a !byItemReadError check so a failed filtered fetch (which leaves stale rows plus the new filters in the slice) still retries on re-entry instead of serving the stale set as a false hit. Filter changes still refetch through handleApply. While in there I also made the thunk drop its whole-set commit if the summit changed under it, since the per-thunk sequence guard is not summit aware. Covered by tests for the skip, the differing-filters refetch, the errored retry, and the summit-switch drop. 038a266.

@caseylocker Σ qty and the "N of M items purchased" badge (purchasedCount, line 147) count canceled lines the same as live ones — item.qty += row.quantity ?? 0 never checks row.is_canceled. Concretely: a sponsor whose only line for an item is a canceled quantity-2 line still shows that item as purchased in the sponsor chip ("1 of 1 items purchased") and contributes 2 to the sponsor-header Σ qty, even though the drill-down row for that same line renders struck-through as canceled.

Reproduced against groupLinesBySponsorItem directly:

const rows = [{
  sponsor: { id: 17, name: "Acme" },
  purchase: { id: 5001, number: "OCP-1", status: "Canceled" },
  item_code: "AV1", description: "Audio mixer",
  quantity: 2, line_total: 100000, is_canceled: true
}];
const [group] = groupLinesBySponsorItem(rows);
group.items[0].qty          // 2  (canceled quantity still counted)
group.purchasedCount        // 1  ("1 of 1 items purchased")
group.totalQty               // 2  (sponsor-header Σ qty also inflated)

I know the canceled-lines-flow-through note in the PR description covers not hiding rows in the drill-down — but that's a different claim from the aggregate badge asserting "purchased." Either exclude canceled lines from qty/purchasedCount (still visible, struck-through, in the drill-down only), or rename the badge to a status-neutral phrase (e.g. "items with activity") if counting canceled activity is intentional.

Went with the exclude option. Canceled lines are now excluded from qty, money, purchasedCount, and the sponsor-header Σ qty; they stay visible struck-through in the contributing-orders drill-down only. I excluded the canceled line_total from the item total as well, so the money and qty columns stay consistent under the same rule. Added a grouping test asserting a canceled-only line reports qty 0 and null money while still appearing as a contributor. 038a266.

I don't think this is a complete solution, two different actions could be calling the same action type, like START_LOADING and have same seq, creating a conflict. Also having to declare one Seq per call is not scalable. @smarcet we should stop building code debt with this and think of a better solution before applying it

Agreed on the scalability point. A seq instance per thunk is boilerplate and won't scale as more bulk loaders land. One possible solution is to pull the whole bulk-load flow (token, pLimit paging, atomic commit, stale guard) into one helper parameterized by url builder, query builder, action types, and a guard instance, then route both this thunk and getSponsorAssetRows through it.

One thing the helper should own explicitly is the summit-validity guard and the terminal loading cleanup, not just the seq. The two thunks differ there on purpose today: by-item guards an in-flight summit switch because its fetch is view-keyed and nothing re-invokes it to bump the seq, while the asset flow is summit-keyed so a switch re-fetches and supersedes the parked call on its own. Folding them together means standardizing that policy across both the commit and the error path rather than copying either shape. It should probably ship as its own small PR, so the refactor of the already-shipped asset loader gets reviewed on its own merits instead of riding inside a view-only change. I'll defer to you and @smarcet.

One clarification on the conflict: each seq is a private closure with its own counter, so isCurrent only ever compares a call against newer calls of the same thunk, never across thunks or action types. Two thunks both sitting at seq 1 don't interact, and a shared START_LOADING can't collide on it, so it's sound as written today. The boilerplate is the real issue and factoring it is the right fix.

All these tests are just testing normal redux logic, we shouldn't create tests like this

Trimmed. Dropped the assertions that only verify a spread copied a field through (the READ_ERROR passthrough and the SET_PAGING field checks), and kept the ones that pin real contracts: summary falling back to null when the envelope omits it, readError clearing on REQUEST/RECEIVE, and the reset to DEFAULT_STATE on SET_CURRENT_SUMMIT and LOGOUT_USER. The SET_PAGING behavior still has integration coverage in the page test. 65bfdc2.

please no nested ternary. @smarcet can we prevent this on eslint ?

no-nested-ternary is already in .eslintrc.json at warn, so these surfaced as warnings rather than errors today. Bumping it to error would trip roughly 19 existing files across the repo, so that's a separate cleanup call for you and @smarcet. For this PR I collapsed the per-view branches (carried, activeSummary, activeFilters, activeReadError, the export handler, and the render block) into a small byView lookup, so the nesting is gone from this file. That also closes the related CodeRabbit note on the same chains. 65bfdc2.

@caseylocker orders and statusMix don't apply the same is_canceled gate as qty/totalCents, so a canceled-only line for an item still counts as a distinct paid order for that item.

Three lines above (96-102), qty and totalCents are correctly wrapped in if (!row.is_canceled) — that's the fix for the earlier "canceled-only line reads as purchased" bug. But item.orderIds.add(purchaseId) and item.statusOrderIds here only check purchaseId != null, not is_canceled, so they still accumulate for a canceled line. When an order has one canceled line and one live line for different items (a case this PR's own description calls out as normal — "line-level soft-canceled rows inside live orders always flow through"), the canceled item's row ends up showing Qty: 0 next to Orders: 1 and a Paid: 1 status chip — which reads as a contradiction next to the just-fixed qty/purchasedCount semantics.

Failing test (drop into the groupLinesBySponsorItem describe block, next to the "EXCLUDES canceled lines..." test):

it("excludes a canceled-only line's order from orders/statusMix, same as qty/money", () => {
  const rows = [
    line({
      item_code: "Z1",
      description: "Canceled only",
      quantity: 4,
      line_total: 40000,
      is_canceled: true,
      purchase: { id: 5001, number: "OCP-1", status: "Paid" }
    })
  ];
  const [group] = groupLinesBySponsorItem(rows);
  const z1 = group.items.find((i) => i.itemCode === "Z1");
  expect(z1.qty).toBe(0);
  expect(z1.totalCents).toBeNull();
  expect(z1.orders).toBe(0); // currently 1
  expect(z1.statusMix).toEqual({}); // currently { Paid: 1 }
});

Is this intentional (e.g. orders/status meant to reflect all order activity regardless of cancellation) or an oversight from the qty/money fix not being extended to these two fields? If intentional, worth a comment here plus a test pinning it — right now nothing distinguishes "designed" from "missed."

Good catch, not intended. That was the qty/money gate not getting extended when I added it. Moved the orderIds and statusMix accumulation inside the same !is_canceled block so all four aggregates (qty, money, orders, status) share one gate. Contributors stay unconditional so canceled lines still show struck-through in the drill-down. Mixed orders keep their count since the live line for the same item re-adds the order id. Added your test with the corrected assertions (orders 0, statusMix {}) and tested locally with the summit 73 cancelled stripe order - STRPTST "Test for Stripe" (FNTECH, order 000010, Paid). Pushed in 1621559.